From cad0afcc0898379d597ad3300d07ed738225ab50 Mon Sep 17 00:00:00 2001 From: Keir Fraser Date: Tue, 22 Apr 2008 11:43:10 +0100 Subject: [PATCH] x86: Fix handling of BSF and BSR instructions. 1. We cannot rely on BSF/BSR leaving the destination register intact if the source is zero (according to Intel manuals) 2. We race clear_bit() in find_first_bit(), which may occur after SCAS but before BSF. So we must handle zero input to BSF. Signed-off-by: Naoki Nishiguchi Signed-off-by: Keir Fraser --- xen/arch/x86/bitops.c | 32 ++++++++++++----------- xen/include/asm-x86/bitops.h | 50 ++++++++++++------------------------ 2 files changed, 33 insertions(+), 49 deletions(-) diff --git a/xen/arch/x86/bitops.c b/xen/arch/x86/bitops.c index 3845f59c96..c037567c6e 100644 --- a/xen/arch/x86/bitops.c +++ b/xen/arch/x86/bitops.c @@ -8,17 +8,18 @@ unsigned int __find_first_bit( unsigned long d0, d1, res; asm volatile ( - " xor %%eax,%%eax\n\t" /* also ensures ZF==1 if size==0 */ + "1: xor %%eax,%%eax\n\t" /* also ensures ZF==1 if size==0 */ " repe; scas"__OS"\n\t" - " je 1f\n\t" + " je 2f\n\t" + " bsf -"STR(BITS_PER_LONG/8)"(%2),%0\n\t" + " jz 1b\n\t" " lea -"STR(BITS_PER_LONG/8)"(%2),%2\n\t" - " bsf (%2),%0\n" - "1: sub %%ebx,%%edi\n\t" + "2: sub %%ebx,%%edi\n\t" " shl $3,%%edi\n\t" " add %%edi,%%eax" : "=&a" (res), "=&c" (d0), "=&D" (d1) - : "1" ((size + BITS_PER_LONG - 1) / BITS_PER_LONG), - "2" (addr), "b" ((int)(long)addr) : "memory" ); + : "1" (BITS_TO_LONGS(size)), "2" (addr), "b" ((int)(long)addr) + : "memory" ); return res; } @@ -34,8 +35,7 @@ unsigned int __find_next_bit( if ( bit != 0 ) { /* Look for a bit in the first word. */ - asm ( "bsf %1,%%"__OP"ax" - : "=a" (set) : "r" (*p >> bit), "0" (BITS_PER_LONG) ); + set = __scanbit(*p >> bit, BITS_PER_LONG - bit); if ( set < (BITS_PER_LONG - bit) ) return (offset + set); offset += BITS_PER_LONG - bit; @@ -56,18 +56,20 @@ unsigned int __find_first_zero_bit( unsigned long d0, d1, d2, res; asm volatile ( + "1: xor %%eax,%%eax ; not %3\n\t" /* rAX == ~0ul */ " xor %%edx,%%edx\n\t" /* also ensures ZF==1 if size==0 */ " repe; scas"__OS"\n\t" - " je 1f\n\t" + " je 2f\n\t" + " xor -"STR(BITS_PER_LONG/8)"(%2),%3\n\t" + " jz 1b\n\t" + " bsf %3,%0\n\t" " lea -"STR(BITS_PER_LONG/8)"(%2),%2\n\t" - " xor (%2),%3\n\t" - " bsf %3,%0\n" - "1: sub %%ebx,%%edi\n\t" + "2: sub %%ebx,%%edi\n\t" " shl $3,%%edi\n\t" " add %%edi,%%edx" : "=&d" (res), "=&c" (d0), "=&D" (d1), "=&a" (d2) - : "1" ((size + BITS_PER_LONG - 1) / BITS_PER_LONG), - "2" (addr), "b" ((int)(long)addr), "3" (-1L) : "memory" ); + : "1" (BITS_TO_LONGS(size)), "2" (addr), "b" ((int)(long)addr) + : "memory" ); return res; } @@ -83,7 +85,7 @@ unsigned int __find_next_zero_bit( if ( bit != 0 ) { /* Look for zero in the first word. */ - asm ( "bsf %1,%%"__OP"ax" : "=a" (set) : "r" (~(*p >> bit)) ); + set = __scanbit(~(*p >> bit), BITS_PER_LONG - bit); if ( set < (BITS_PER_LONG - bit) ) return (offset + set); offset += BITS_PER_LONG - bit; diff --git a/xen/include/asm-x86/bitops.h b/xen/include/asm-x86/bitops.h index 5143bb525b..d3664e3d4c 100644 --- a/xen/include/asm-x86/bitops.h +++ b/xen/include/asm-x86/bitops.h @@ -331,10 +331,9 @@ extern unsigned int __find_first_zero_bit( extern unsigned int __find_next_zero_bit( const unsigned long *addr, unsigned int size, unsigned int offset); -/* return index of first bit set in val or BITS_PER_LONG when no bit is set */ -static inline unsigned int __scanbit(unsigned long val) +static inline unsigned int __scanbit(unsigned long val, unsigned long max) { - asm ( "bsf %1,%0" : "=r" (val) : "r" (val), "0" (BITS_PER_LONG) ); + asm ( "bsf %1,%0 ; cmovz %2,%0" : "=&r" (val) : "r" (val), "r" (max) ); return (unsigned int)val; } @@ -346,9 +345,9 @@ static inline unsigned int __scanbit(unsigned long val) * Returns the bit-number of the first set bit, not the number of the byte * containing a bit. */ -#define find_first_bit(addr,size) \ -((__builtin_constant_p(size) && (size) <= BITS_PER_LONG ? \ - (__scanbit(*(const unsigned long *)addr)) : \ +#define find_first_bit(addr,size) \ +((__builtin_constant_p(size) && (size) <= BITS_PER_LONG ? \ + (__scanbit(*(const unsigned long *)addr, size)) : \ __find_first_bit(addr,size))) /** @@ -357,9 +356,9 @@ static inline unsigned int __scanbit(unsigned long val) * @offset: The bitnumber to start searching at * @size: The maximum size to search */ -#define find_next_bit(addr,size,off) \ -((__builtin_constant_p(size) && (size) <= BITS_PER_LONG ? \ - ((off) + (__scanbit((*(const unsigned long *)addr) >> (off)))) : \ +#define find_next_bit(addr,size,off) \ +((__builtin_constant_p(size) && (size) <= BITS_PER_LONG ? \ + ((off) + (__scanbit((*(const unsigned long *)addr) >> (off), size))) : \ __find_next_bit(addr,size,off))) /** @@ -370,9 +369,9 @@ static inline unsigned int __scanbit(unsigned long val) * Returns the bit-number of the first zero bit, not the number of the byte * containing a bit. */ -#define find_first_zero_bit(addr,size) \ -((__builtin_constant_p(size) && (size) <= BITS_PER_LONG ? \ - (__scanbit(~*(const unsigned long *)addr)) : \ +#define find_first_zero_bit(addr,size) \ +((__builtin_constant_p(size) && (size) <= BITS_PER_LONG ? \ + (__scanbit(~*(const unsigned long *)addr, size)) : \ __find_first_zero_bit(addr,size))) /** @@ -381,9 +380,9 @@ static inline unsigned int __scanbit(unsigned long val) * @offset: The bitnumber to start searching at * @size: The maximum size to search */ -#define find_next_zero_bit(addr,size,off) \ -((__builtin_constant_p(size) && (size) <= BITS_PER_LONG ? \ - ((off)+(__scanbit(~(((*(const unsigned long *)addr)) >> (off))))) : \ +#define find_next_zero_bit(addr,size,off) \ +((__builtin_constant_p(size) && (size) <= BITS_PER_LONG ? \ + ((off)+(__scanbit(~(((*(const unsigned long *)addr)) >> (off)), size))) : \ __find_next_zero_bit(addr,size,off))) @@ -391,8 +390,7 @@ static inline unsigned int __scanbit(unsigned long val) * find_first_set_bit - find the first set bit in @word * @word: the word to search * - * Returns the bit-number of the first set bit. If no bits are set then the - * result is undefined. + * Returns the bit-number of the first set bit. The input must *not* be zero. */ static inline unsigned int find_first_set_bit(unsigned long word) { @@ -400,27 +398,11 @@ static inline unsigned int find_first_set_bit(unsigned long word) return (unsigned int)word; } -/** - * ffz - find first zero in word. - * @word: The word to search - * - * Undefined if no zero exists, so code should check against ~0UL first. - */ -static inline unsigned long ffz(unsigned long word) -{ - asm ( "bsf %1,%0" - :"=r" (word) - :"r" (~word)); - return word; -} - /** * ffs - find first bit set * @x: the word to search * - * This is defined the same way as - * the libc and compiler builtin ffs routines, therefore - * differs in spirit from the above ffz (man ffs). + * This is defined the same way as the libc and compiler builtin ffs routines. */ static inline int ffs(unsigned long x) { -- 2.30.2